feat(ci): surface merge-queue needs-validation ejection as a PR comment - #5519
Conversation
A PR ejected from the merge queue by the needs-validation gate leaves no trace on the PR itself: the gate fails on the queue's transient ref, the PR's own checks stay green, and mergeQueueEntry silently goes null. The only way to learn why the PR fell out of the queue is to dig through merge_group workflow runs, which nobody watches (observed on #5329). Producer: the merge_group gate in ci.yml now emits a handoff/comment artifact targeting the labeled PR right before it blocks, and uploads it on the failure path. Consumer: comment.atom.yml accepts merge_group workflow_run events; since a merge_group run's head_sha is the queue's synthetic merge commit, artifacts from those runs are bound to their producing run by run_id, and the base-freshness skip is waived (PRs ahead in the queue move the base while the run completes). The live-PR head check still guards staleness. Topology coverage pins the new boundary: comment.atom consumes pull_request + merge_group; autofix/report stay pull_request-only.
lefarcen
added
size/M
|
Quick template nit: the motivation and rollout details are clear, but the PR body is still missing a The existing |
lefarcen
added
the
skip-validation
PerishCode
left a comment
PerishCode
left a comment
There was a problem hiding this comment.
@lefarcen I reviewed the workflow and topology-test changes for the merge-queue needs-validation ejection path. The producer keeps the existing merge_group gate semantics intact while emitting a validated handoff/comment artifact on the blocking path, and the comment atom’s merge_group handling still keeps the live PR head/state/draft checks in place while avoiding the synthetic queue head/base mismatch. The e2e topology coverage also pins the intended consumer boundary so autofix/report stay pull_request-only.
No actionable findings from this pass. Nice work making the queue ejection visible without changing the gate’s required-check behavior.
🔁 Powered by Looper · runner=reviewer · agent=codex · An autonomous AI dev team for your GitHub repos.
Visual regression reviewHead: 1 changed · 52 unchanged · 0 new without baseline · 0 failed Changed cases
Unchanged cases
Visual diff is advisory only and does not block merging. |
2 participants
Why
While driving #5329 to merge we hit the needs-validation gate's blind spot first-hand: the PR was approved, every check on the head was green,
enqueuePullRequestsucceeded — and then the PR silently fell out of the merge queue. Nothing on the PR said why.mergeQueueEntryjust went null, the head stayedCLEAN, and the actual reason (PR #5329 still has 'needs-validation' — blocking merge) lived in amerge_groupworkflow run that no surface links back to the PR. The gate's own design comment acknowledges this ("this failure does NOT appear in the PR head's status rollup") — that is correct and intentional for keeping PR checks green, but it currently means every ejection costs someone a manual dig throughgh run list --event merge_group.This closes that gap: an ejection now leaves a visible, actionable trace on the PR itself.
What users will see
When a queued PR is ejected because it still carries the
needs-validationlabel, the PR gets a bot comment (upserted, so repeated ejections update one comment instead of stacking):No change for PRs without the label, and no change to the gate's blocking semantics.
How
Follows the
ci.ymlproducer +comment.atom.ymlconsumer topology from.github/AGENTS.md:ci.yml,validatejob): the merge_group gate emits ahandoff/commentartifact via.github/scripts/handoff.pyfor the labeled PR right beforeexit 1, and anif: failure() && …step uploads it (the gate fails exactly when it produces). A sparse checkout of.github(merge_group runs only) makes the helper available; production is best-effort and can never soften the block itself. Fail-fast keeps one notice per run — every labeled entry also fails its own queue run, so each still gets its own notice.comment.atom.yml): now acceptsmerge_groupworkflow_run events. A merge_group run'shead_shais the queue's synthetic merge commit, never the PR head, so artifacts from those runs are bound to their producing run byrun_id(already part of the handoff contract — nohandoff.pychanges needed). The base-freshness skip is waived for merge_group (PRs ahead in the queue merge into the base while the run completes, so the base is expected to move); the live-PR head/state/draft checks still guard staleness.autofix.atom.ymlandreport.atom.ymldeliberately stay pull_request-only.e2e/tests/packaged-smoke-workflow.test.ts): the trusted-consumer test now pins comment.atom topull_request || merge_groupand asserts autofix/report do not consume merge_group; a new case covers the producer/consumer pair for the ejection notice.Surface area
Test plan
python3 .github/scripts/handoff.py self-checkactionlinton both modified workflows — cleanbash -non both extractedrun:blockshandoff.py validate comment, and the consumer-sidelistoutput (includingrun_id) all round-trip; body renders with the correct run linkpnpm --filter @open-design/e2e test tests/packaged-smoke-workflow.test.ts— 46/46 (includes the new topology case)pnpm guard(78/0) andpnpm typecheckReal-world validation happens on the first ejection after merge; the motivating repro is #5329's ejection run https://github.com/nexu-io/open-design/actions/runs/29222046956.